Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix song-info callback duplication #269

Merged
merged 5 commits into from
May 28, 2021

Conversation

Araxeus
Copy link
Collaborator

@Araxeus Araxeus commented May 10, 2021

Old behavior

Every plugin had to setup SongInfo separately

Which causes duplication of listener registration win.on("page-title-updated"... and ipcMain.on("song-info-request"...

Which causes song-info callbacks to be called multiple times
(because ipcMain callbacks were registered X times and sent X events for each "song-info-request")

New behavior

songInfo is setup once from index.js before plugin load.
Each plugin can register a song-info callback by just requiring it. (without the need to set it up)

achieved by separating registerProvider() and registerCallback() in song-info.js

registerCallback example:

const registerCallback = require("../../providers/song-info");
registerCallback(songInfo => {
	console.log(songInfo.title);
})

This fix also warranted a small refactor of notifications plugin (which was the most heavily affected by this bug)

@Araxeus Araxeus changed the title fix song-info callback being called twice fix song-info callback duplication May 10, 2021
Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Change looks good overall but is it expected that the downloader menu import is not updated as well?
https:/th-ch/youtube-music/blob/master/plugins/downloader/menu.js#L11
(I get TypeError: registerCallback is not a function when launching the app)

@Araxeus
Copy link
Collaborator Author

Araxeus commented May 18, 2021

Thanks for the fix! Change looks good overall but is it expected that the downloader menu import is not updated as well?
master/plugins/downloader/menu.js#L11
(I get TypeError: registerCallback is not a function when launching the app)

Sorry I don't know how I missed this one. it should be fine now

Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the improvement!

@th-ch th-ch merged commit 3515bf3 into th-ch:master May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants